Skip to content

Prepare the driver to work with newer server releases than its own #215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 10, 2020

Conversation

bpintea
Copy link
Collaborator

@bpintea bpintea commented Mar 9, 2020

This PR relaxes the version check the driver does on connect: this
version compatibility is migrated on the server and the driver only
checks that server's version is more recent than a set minimum (that of
the release that applies version policy on the server).

(Related to elastic/elasticsearch#53082)

bpintea added 3 commits March 4, 2020 22:07
This commit relaxes the version check the driver does on connect: this
version compatibility is migrated on the server and the driver only
checks that server's version is more recent than a set minimum (that of
the release that applies version policy on the server).
remove superfluous defines
This commit adds the newly required 'version' field into the REST
request objects.
bpintea added 2 commits March 10, 2020 08:11
This commit fixes two issues:
- the "version" field is only required for "query" requst objects, but
not needed for "cursor" (paginating) ones;
- the lenght of the version field value is now correctly estimated in
CBOR encoding.
Update position of the "version" field in expected output of the request
object.
Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

But since the PR title goes into the release notes it might be best to change it to something like "Make ODBC drivers work with a wider range of server versions" or something like that. If I was a user suffering from the strict version compatibility checking then seeing that in the release notes would tell me my problem was solved. (If there is going to be another PR that completes the transition and has a title like that then this one can stay as-is.)

@bpintea bpintea changed the title Transfer version compatibility check onto the server Prepare the driver to work with newer server releases than its own Mar 10, 2020
@bpintea
Copy link
Collaborator Author

bpintea commented Mar 10, 2020

Thanks for the suggestion, title updated.

@bpintea bpintea merged commit 8b045b1 into elastic:master Mar 10, 2020
@bpintea bpintea deleted the enh/relax_version_check branch March 10, 2020 08:42
bpintea added a commit that referenced this pull request Mar 17, 2020
)

* relax server's version checking

This commit relaxes the version check the driver does on connect: this
version compatibility is migrated on the server and the driver only
checks that server's version is more recent than a set minimum (that of
the release that applies version policy on the server).

* remove unused defines

remove superfluous defines

* add 'version' field into request objects

This commit adds the newly required 'version' field into the REST
request objects.

* only add the "version" to "query" objects

This commit fixes two issues:
- the "version" field is only required for "query" requst objects, but
not needed for "cursor" (paginating) ones;
- the lenght of the version field value is now correctly estimated in
CBOR encoding.

* update unit tests to "version" in query-req only

Update position of the "version" field in expected output of the request
object.

(cherry picked from commit 8b045b1)
bpintea added a commit to bpintea/elasticsearch-sql-odbc that referenced this pull request Mar 23, 2020
This commit adds a new feature that allows the user to limit the size of
the string types (TEXT, KEYWORD & co.). This is needed for those
applications that won't accept the large maxes that ES does for the
fields of these types.

The change will manifest itself as:
- limitation of the COLUMN_SIZE, BUFFER_LENGHT and CHAR_OCTET_LENGTH in
the SQLColumns output;
- limitation of the "display size" attribute in the description of the
columns of the above mentioned types;
- truncation of the results to the configured limit for the columns of
the above mentioned types.

The commit also adds unit tests for the "faked" (i.e. driver-returned
results) catalog functions, removes stale DSN attribute definition (left
over from elastic#215) and corrects the row index in a debug statement.
bpintea added a commit that referenced this pull request Mar 24, 2020
* introduce new varchar limitation option

This commit adds a new feature that allows the user to limit the size of
the string types (TEXT, KEYWORD & co.). This is needed for those
applications that won't accept the large maxes that ES does for the
fields of these types.

The change will manifest itself as:
- limitation of the COLUMN_SIZE, BUFFER_LENGHT and CHAR_OCTET_LENGTH in
the SQLColumns output;
- limitation of the "display size" attribute in the description of the
columns of the above mentioned types;
- truncation of the results to the configured limit for the columns of
the above mentioned types.

The commit also adds unit tests for the "faked" (i.e. driver-returned
results) catalog functions, removes stale DSN attribute definition (left
over from #215) and corrects the row index in a debug statement.

* add new integration testing running mode

Add new option to execute only the tests requiring no indexing of data
(currently server info-fetching tests and the protocol tests).

The commit also sets the autocommit as parameter to connection function,
instead of the subsequent operation.
bpintea added a commit that referenced this pull request Mar 30, 2020
* introduce new varchar limitation option

This commit adds a new feature that allows the user to limit the size of
the string types (TEXT, KEYWORD & co.). This is needed for those
applications that won't accept the large maxes that ES does for the
fields of these types.

The change will manifest itself as:
- limitation of the COLUMN_SIZE, BUFFER_LENGHT and CHAR_OCTET_LENGTH in
the SQLColumns output;
- limitation of the "display size" attribute in the description of the
columns of the above mentioned types;
- truncation of the results to the configured limit for the columns of
the above mentioned types.

The commit also adds unit tests for the "faked" (i.e. driver-returned
results) catalog functions, removes stale DSN attribute definition (left
over from #215) and corrects the row index in a debug statement.

* add new integration testing running mode

Add new option to execute only the tests requiring no indexing of data
(currently server info-fetching tests and the protocol tests).

The commit also sets the autocommit as parameter to connection function,
instead of the subsequent operation.

(cherry picked from commit 77dc4ff)
bpintea added a commit that referenced this pull request Mar 30, 2020
* introduce new varchar limitation option

This commit adds a new feature that allows the user to limit the size of
the string types (TEXT, KEYWORD & co.). This is needed for those
applications that won't accept the large maxes that ES does for the
fields of these types.

The change will manifest itself as:
- limitation of the COLUMN_SIZE, BUFFER_LENGHT and CHAR_OCTET_LENGTH in
the SQLColumns output;
- limitation of the "display size" attribute in the description of the
columns of the above mentioned types;
- truncation of the results to the configured limit for the columns of
the above mentioned types.

The commit also adds unit tests for the "faked" (i.e. driver-returned
results) catalog functions, removes stale DSN attribute definition (left
over from #215) and corrects the row index in a debug statement.

* add new integration testing running mode

Add new option to execute only the tests requiring no indexing of data
(currently server info-fetching tests and the protocol tests).

The commit also sets the autocommit as parameter to connection function,
instead of the subsequent operation.

(cherry picked from commit 77dc4ff)
@bpintea bpintea mentioned this pull request Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants